Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add a new label for scraping metrics #1190

Merged
merged 6 commits into from
Dec 1, 2020
Merged

Add a new label for scraping metrics #1190

merged 6 commits into from
Dec 1, 2020

Conversation

kasia-kujawa
Copy link
Contributor

Description

Fixes #896

Testing performed
  • ci/build.sh
  • Redeploy fluentd and fluentd-events pods
  • Confirm events, logs, and metrics are coming in

@kasia-kujawa kasia-kujawa requested a review from a team November 30, 2020 09:24
sumo-drosiek
sumo-drosiek suggested changes Nov 30, 2020
View reviewed changes
Copy link
Contributor

@sumo-drosiek sumo-drosiek left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add sumologic/scrape: true annotation instead of name based. In the prometheus we can set multiple matchLabels to scrape by, so it will be eg.:

          matchLabels:
            app.kubernetes.io/name: fluent-bit
            sumologic.com/scrape: true

@kasia-kujawa
Copy link
Contributor Author

I would add sumologic/scrape: true annotation instead of name based. In the prometheus we can set multiple matchLabels to scrape by, so it will be eg.:

          matchLabels:
            app.kubernetes.io/name: fluent-bit
            sumologic.com/scrape: true

Annotations are not used to identify and select objects. , see this.

I tested with annotations and it does not work, target for metics is not correctly selected.
My changes:

% git diff
diff --git a/deploy/helm/sumologic/values.yaml b/deploy/helm/sumologic/values.yaml
index f67d22f9..040e147d 100644
--- a/deploy/helm/sumologic/values.yaml
+++ b/deploy/helm/sumologic/values.yaml
@@ -772,9 +772,8 @@ fluent-bit:
 
   ## Configuration for fluent-bit service
   service:
-    ## Add custom labels to fluent-bit service
-    labels:
-      sumologic.com/scrape: fluent-bit
+    annotations:
+      sumologic.com/scrape: "true"
 
   ## Add custom pod labels to fluent-bit daemonset pods
   podLabels: {}
@@ -1063,7 +1062,8 @@ kube-prometheus-stack:
             - $(NAMESPACE)
         selector:
           matchLabels:
-            sumologic.com/scrape: fluent-bit
+            app.kubernetes.io/name: fluent-bit
+            sumologic.com/scrape: "true"
       - name: collection-sumologic-otelcol
         additionalLabels:
           sumologic.com/app: otelcol

Created resources:

$ kubectl describe servicemonitors collection-fluent-bit
Name:         collection-fluent-bit
Namespace:    sumologic
Labels:       app=collection-fluent-bit
              app.kubernetes.io/managed-by=Helm
              chart=kube-prometheus-stack-12.0.2
              heritage=Helm
              release=collection
Annotations:  meta.helm.sh/release-name: collection
              meta.helm.sh/release-namespace: sumologic
API Version:  monitoring.coreos.com/v1
Kind:         ServiceMonitor
Metadata:
  Creation Timestamp:  2020-11-30T08:19:00Z
  Generation:          5
  Managed Fields:
    API Version:  monitoring.coreos.com/v1
    Fields Type:  FieldsV1
    fieldsV1:
      f:metadata:
        f:annotations:
          .:
          f:meta.helm.sh/release-name:
          f:meta.helm.sh/release-namespace:
        f:labels:
          .:
          f:app:
          f:app.kubernetes.io/managed-by:
          f:chart:
          f:heritage:
          f:release:
      f:spec:
        .:
        f:endpoints:
        f:namespaceSelector:
          .:
          f:matchNames:
        f:selector:
          .:
          f:matchLabels:
            .:
            f:app.kubernetes.io/name:
            f:sumologic.com/scrape:
    Manager:         Go-http-client
    Operation:       Update
    Time:            2020-11-30T10:06:19Z
  Resource Version:  18397
  Self Link:         /apis/monitoring.coreos.com/v1/namespaces/sumologic/servicemonitors/collection-fluent-bit
  UID:               04df55e7-7f71-4228-a1e5-53435c99079c
Spec:
  Endpoints:
    Path:  /api/v1/metrics/prometheus
    Port:  http
  Namespace Selector:
    Match Names:
      $(NAMESPACE)
  Selector:
    Match Labels:
      app.kubernetes.io/name:  fluent-bit
      sumologic.com/scrape:    true
Events:                        <none>



$ kubectl describe svc collection-fluent-bit
Name:              collection-fluent-bit
Namespace:         sumologic
Labels:            app.kubernetes.io/instance=collection
                   app.kubernetes.io/managed-by=Helm
                   app.kubernetes.io/name=fluent-bit
                   app.kubernetes.io/version=1.6.3
                   helm.sh/chart=fluent-bit-0.7.4
Annotations:       meta.helm.sh/release-name: collection
                   meta.helm.sh/release-namespace: sumologic
                   sumologic.com/scrape: true
Selector:          app.kubernetes.io/instance=collection,app.kubernetes.io/name=fluent-bit
Type:              ClusterIP
IP:                10.152.183.87
Port:              http  2020/TCP
TargetPort:        http/TCP
Endpoints:         10.1.57.18:2020
Session Affinity:  None
Events:            <none>

@kasia-kujawa
Copy link
Contributor Author

I'll add label "sumologic.com/scrape: true"

@kasia-kujawa kasia-kujawa force-pushed the kkujawa-add-label-scrape branch 2 times, most recently from 46858d1 to 2c74dcd Compare November 30, 2020 12:26
sumo-drosiek
sumo-drosiek reviewed Nov 30, 2020
View reviewed changes
deploy/helm/sumologic/values.yaml Outdated
service:
## Add custom labels to fluent-bit service
labels:
sumologic.com/scrape: "true" # label used by fluent-bit service monitor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't be this way? 🤔

Suggested change
sumologic.com/scrape: "true" # label used by fluent-bit service monitor
sumologic.com/scrape: "true" # label used by fluent-bit service monitor

@kasia-kujawa kasia-kujawa force-pushed the kkujawa-add-label-scrape branch from 2c74dcd to 10bb43c Compare November 30, 2020 12:28
@kasia-kujawa kasia-kujawa force-pushed the kkujawa-add-label-scrape branch from 10bb43c to 3fcd985 Compare November 30, 2020 12:30
@kasia-kujawa kasia-kujawa merged commit b6fd709 into main Dec 1, 2020
@kasia-kujawa kasia-kujawa deleted the kkujawa-add-label-scrape branch December 1, 2020 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmalek-sumo pmalek-sumo pmalek-sumo approved these changes

@sumo-drosiek sumo-drosiek sumo-drosiek approved these changes

@andrzej-stencel andrzej-stencel Awaiting requested review from andrzej-stencel

@frankreno frankreno Awaiting requested review from frankreno

@perk-sumo perk-sumo Awaiting requested review from perk-sumo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Split internal sumo labels for tagging services and scraping metrics from those services
3 participants
@kasia-kujawa @sumo-drosiek @pmalek-sumo